SMQ-2627 - Align PATs with new architecture #3295

nyagamunene · 2025-12-18T16:42:54Z

What type of PR is this?

What does this do?

Which issue(s) does this PR fix/relate to?

Related Issue Feature: PAT align with new architecture #2627
Resolves Feature: PAT align with new architecture #2627

Have you included tests for your changes?

Did you document any new/modified feature?

Notes

codecov · 2025-12-18T16:54:23Z

Codecov Report

❌ Patch coverage is 64.60674% with 63 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.50%. Comparing base (28fc973) to head (a04e9ea).

Files with missing lines	Patch %	Lines
auth/postgres/repo.go	0.00%	31 Missing ⚠️
auth/postgres/pat.go	0.00%	12 Missing ⚠️
auth/cache/pat.go	0.00%	6 Missing ⚠️
auth/pat.go	82.14%	3 Missing and 2 partials ⚠️
auth/api/grpc/auth/server.go	84.61%	2 Missing and 2 partials ⚠️
auth/service.go	0.00%	3 Missing ⚠️
auth/api/grpc/auth/client.go	95.55%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #3295       +/-   ##
===========================================
+ Coverage   53.21%   74.50%   +21.28%     
===========================================
  Files         312      179      -133     
  Lines       32706    19778    -12928     
===========================================
- Hits        17405    14735     -2670     
+ Misses      14234     4170    -10064     
+ Partials     1067      873      -194

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

dborovcanin · 2025-12-23T10:15:36Z

pkg/authn/authsvc/authn.go

 	}

-	if strings.HasPrefix(token, patPrefix) {
+	tokenType := authn.TokenType(res.GetTokenType())


This creates possible misconfiguration issue. I.e. what if token prefix here and tokenType returned from the service are not the same? While we are not able to validate token - we are able to parse it locally (even if only for prefix) and we can do this verification to prevent mismatch.

dborovcanin · 2025-12-23T10:18:32Z

channels/middleware/authorization.go

-			} else {
-				req.Operation = auth.ListOp
-			}
+			req.Operation = auth.ChannelDisconnectClientOp


Let's stay consistent in naming, compare this to ClientConnectToChannelOp.

arvindh123 · 2025-12-23T12:07:36Z

auth/pat.go

-	UnshareOp
-	PublishOp
-	SubscribeOp
+	ClientCreateOp Operation = iota + 100


Is it possible to reuse the existing service Operations ?

we will have cyclic imports

felixgateru · 2026-01-12T10:16:42Z

auth/pat.go

 	return err
 }

+var ValidOperationsForEntity = map[EntityType][]Operation{


I noticed here that we have not included domains as an entity, does this mean that pats can not be used for actions on domains?

From past discussion with @arvindh123, PATs are being used for entities like channels, clients, groups, dashboards and messages. It is not used to create users and domains. @dborovcanin can you confirm this?

This is fine; we can limit PATS to this at the moment. I.e. PATS are used for users to manage entities within the domain.

felixgateru · 2026-01-12T11:21:37Z

auth/pat.go

 	}

+	if !IsValidOperationForEntity(s.EntityType, s.Operation) {
+		return errors.Wrap(apiutil.ErrInvalidQueryParams, errors.New("operation not valid for entity type"))


You can define this error, instead of calling new each time

internal/proto/auth/v1/auth.proto

dborovcanin · 2026-01-12T12:48:04Z

internal/proto/auth/v1/auth.proto

+  string user_id = 2;             // User id
+  string pat_id = 3;              // Pat id
+  uint32 entity_type = 4;         // Entity type
+  string optional_domain_id = 5;  // Optional domain id


Why not optional string domain_id = 5?

Since users was remove I think it should be optional

arvindh123 · 2026-01-14T14:15:39Z

pkg/roles/rolemanager/middleware/authorization.go


+	if pr.PatID != "" && pr.TokenType == authn.PersonalAccessToken {
+		pr.EntityID = pr.Object
+
+		switch ram.entityType {
+		case policies.ClientType:
+			pr.EntityType = auth.ClientsType
+		case policies.ChannelType:
+			pr.EntityType = auth.ChannelsType
+		case policies.GroupType:
+			pr.EntityType = auth.GroupsType
+		}
+
+		switch op {
+		case roles.OpAddRole:
+			pr.Operation = auth.RoleAddOp
+		case roles.OpRemoveRole:
+			pr.Operation = auth.RoleRemoveOp
+		case roles.OpUpdateRoleName:
+			pr.Operation = auth.RoleUpdateOp
+		case roles.OpRetrieveRole:
+			pr.Operation = auth.RoleRetrieveOp
+		case roles.OpRetrieveAllRoles:
+			pr.Operation = auth.RoleRetrieveAllOp
+		case roles.OpRoleAddActions:
+			pr.Operation = auth.RoleAddActionsOp
+		case roles.OpRoleListActions:
+			pr.Operation = auth.RoleListActionsOp
+		case roles.OpRoleCheckActionsExists:
+			pr.Operation = auth.RoleCheckActionsExistsOp
+		case roles.OpRoleRemoveActions:
+			pr.Operation = auth.RoleRemoveActionsOp
+		case roles.OpRoleRemoveAllActions:
+			pr.Operation = auth.RoleRemoveAllActionsOp
+		case roles.OpRoleAddMembers:
+			pr.Operation = auth.RoleAddMembersOp
+		case roles.OpRoleListMembers:
+			pr.Operation = auth.RoleListMembersOp
+		case roles.OpRoleCheckMembersExists:
+			pr.Operation = auth.RoleCheckMembersExistsOp
+		case roles.OpRoleRemoveMembers:
+			pr.Operation = auth.RoleRemoveMembersOp
+		case roles.OpRoleRemoveAllMembers:
+			pr.Operation = auth.RoleRemoveAllMembersOp
+		}
+	}


I suspect this code need , These should be passed from permissions.yaml file

arvindh123 · 2026-01-14T14:42:04Z

internal/proto/auth/v1/auth.proto

 message PolicyReq {
-  uint32 token_type = 1;          // Token type
-  string domain = 2;              // Domain
-  string subject_type = 3;        // Client or User
-  string subject_kind = 4;        // ID or Token
-  string subject_relation = 5;    // Subject relation
-  string subject = 6;             // Subject value
-  string relation = 7;            // Relation to filter
-  string permission = 8;          // Action
-  string object = 9;              // Object ID
-  string object_type = 10;        // Client, User, Group
+  uint32 token_type = 1;
+  string domain = 2;
+  string subject_type = 3;
+  string subject_kind = 4;
+  string subject_relation = 5;
+  string subject = 6;
+  string relation = 7;
+  string permission = 8;
+  string object = 9;
+  string object_type = 10;
 }


Is it possible to fit the PATreq within here? Because we have token_type here

auth/postgres/init.go

auth/pat.go

auth/scope_test.go

auth/service.go

Signed-off-by: nyagamunene <[email protected]>

arvindh123

I have idea now, no need to implement in this PR.

We can have the permission map in auth service only
And in policy request we can send only the operations

Since PAT also sends the operation , we can make the users authz to send the operation instead of permission and
In auth service we can convert operations to Permissions

What do you think?

arvindh123 · 2026-01-20T12:53:33Z

internal/proto/auth/v1/auth.proto

+  string permission = 7;
+  string object = 8;
+  string object_type = 9;
+  string user_id = 10;


Is it possible to use the subject_type and subject instead of this user id ?

arvindh123 · 2026-01-20T12:54:31Z

internal/proto/auth/v1/auth.proto

+  string object_type = 9;
+  string user_id = 10;
+  string pat_id = 11;
+  uint32 entity_type = 12;


Is it possible to use the object_type instead of entity_type ?

arvindh123 · 2026-01-20T12:54:55Z

internal/proto/auth/v1/auth.proto

+  string pat_id = 11;
+  uint32 entity_type = 12;
+  uint32 operation = 13;
+  string entity_id = 14;


Is it possible to use the object instead of entity_id ?

Signed-off-by: nyagamunene <[email protected]>

nyagamunene marked this pull request as ready for review December 18, 2025 21:06

nyagamunene requested a review from a team as a code owner December 18, 2025 21:06

nyagamunene force-pushed the SMQ-2627 branch from 7aa84c3 to 8976860 Compare December 22, 2025 18:55

dborovcanin requested changes Dec 23, 2025

View reviewed changes

arvindh123 requested changes Dec 23, 2025

View reviewed changes

nyagamunene force-pushed the SMQ-2627 branch from d2f93f8 to 9632e74 Compare December 23, 2025 12:47

dborovcanin force-pushed the SMQ-2627 branch from 81bc751 to 30bd274 Compare December 23, 2025 15:05

nyagamunene force-pushed the SMQ-2627 branch from 30bd274 to 55ad6f9 Compare January 12, 2026 08:53

felixgateru reviewed Jan 12, 2026

View reviewed changes

felixgateru requested changes Jan 12, 2026

View reviewed changes

felixgateru previously approved these changes Jan 12, 2026

View reviewed changes

dborovcanin requested changes Jan 12, 2026

View reviewed changes

nyagamunene dismissed felixgateru’s stale review via d284ac7 January 12, 2026 13:10

arvindh123 requested changes Jan 14, 2026

View reviewed changes

nyagamunene force-pushed the SMQ-2627 branch from bb5595c to cdb0867 Compare January 15, 2026 08:14

felixgateru requested changes Jan 15, 2026

View reviewed changes

auth/postgres/init.go Show resolved Hide resolved

auth/pat.go Show resolved Hide resolved

auth/scope_test.go Show resolved Hide resolved

auth/service.go Outdated Show resolved Hide resolved

nyagamunene force-pushed the SMQ-2627 branch 2 times, most recently from a7d4bd1 to 7a0353a Compare January 19, 2026 10:50

nyagamunene added 11 commits January 19, 2026 14:22

align pats with entity operations

1e96915

Signed-off-by: nyagamunene <[email protected]>

fix authorization

7bc9c0a

Signed-off-by: nyagamunene <[email protected]>

fix tests

2eece87

Signed-off-by: nyagamunene <[email protected]>

fix failing linter

39b307f

Signed-off-by: nyagamunene <[email protected]>

update proto

00b81d0

Signed-off-by: nyagamunene <[email protected]>

update proto

cd35f3f

Signed-off-by: nyagamunene <[email protected]>

address comments

1bd0a27

Signed-off-by: nyagamunene <[email protected]>

address comments

4fba6c5

Signed-off-by: nyagamunene <[email protected]>

update error

3b5d738

Signed-off-by: nyagamunene <[email protected]>

fix failing linter

e3dc542

Signed-off-by: nyagamunene <[email protected]>

fix pats tests

8ff32b2

Signed-off-by: nyagamunene <[email protected]>

nyagamunene added 8 commits January 19, 2026 14:22

define error as a variable

9806107

Signed-off-by: nyagamunene <[email protected]>

address comments

e76fbec

Signed-off-by: nyagamunene <[email protected]>

fix failing linter

622c6cc

Signed-off-by: nyagamunene <[email protected]>

fix failing linter

2ede173

Signed-off-by: nyagamunene <[email protected]>

address comments

f78a248

Signed-off-by: nyagamunene <[email protected]>

refactor permissions.yaml

7e84885

Signed-off-by: nyagamunene <[email protected]>

fix roles permissions

98c0b36

Signed-off-by: nyagamunene <[email protected]>

address comments

7937796

Signed-off-by: nyagamunene <[email protected]>

nyagamunene force-pushed the SMQ-2627 branch from 7a0353a to 7937796 Compare January 19, 2026 11:22

address comments

08ca154

Signed-off-by: nyagamunene <[email protected]>

felixgateru previously approved these changes Jan 20, 2026

View reviewed changes

arvindh123 requested changes Jan 20, 2026

View reviewed changes

refactor proto file

a04e9ea

Signed-off-by: nyagamunene <[email protected]>

nyagamunene dismissed felixgateru’s stale review via a04e9ea January 20, 2026 16:06

SMQ-2627 - Align PATs with new architecture #3295

Are you sure you want to change the base?

SMQ-2627 - Align PATs with new architecture #3295

Uh oh!

Conversation

nyagamunene commented Dec 18, 2025

What type of PR is this?

What does this do?

Which issue(s) does this PR fix/relate to?

Have you included tests for your changes?

Did you document any new/modified feature?

Notes

Uh oh!

codecov bot commented Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

arvindh123 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov bot commented Dec 18, 2025 •

edited

Loading